Skip to content

feat(results): expose Tekton Results Watcher config via TektonConfig#3709

Open
adchauha wants to merge 4 commits into
tektoncd:mainfrom
adchauha:SRVKP-11569-watcher-tektonconfig
Open

feat(results): expose Tekton Results Watcher config via TektonConfig#3709
adchauha wants to merge 4 commits into
tektoncd:mainfrom
adchauha:SRVKP-11569-watcher-tektonconfig

Conversation

@adchauha

@adchauha adchauha commented Jul 13, 2026

Copy link
Copy Markdown

Add spec.result.watcher so watcher flags can be configured through TektonConfig without manually patching tekton-results-watcher Deployment args.

Changes

  • Add ResultsWatcherProperties and spec.result.watcher to TektonConfig and TektonResult CRDs
  • Wire watcher configuration into tekton-results-watcher Deployment container args via UpdateWatcherFlagsInDeployment
  • Add validation for watcher fields (label selector syntax, non-negative durations)
  • Regenerate CRDs and deepcopy codegen
  • Add unit tests for watcher validation and deployment arg transformation
  • Document watcher configuration in docs/TektonConfig.md and docs/TektonResult.md

Watcher flags exposed include:

  • completed_run_grace_period
  • check_owner
  • store_deadline
  • disable_storing_incomplete_runs
  • logs_api, logs_timestamps, store_event
  • summary_labels, summary_annotations, label_selector
  • requeue_interval, forward_buffer, update_log_timeout, dynamic_reconcile_timeout, disable_crd_update

Submitter Checklist

These are the criteria that every PR should meet, please check them off as you
review them:

See the contribution guide for more details.

Release Notes

TektonConfig and TektonResult now support `spec.result.watcher` to configure Tekton Results Watcher behavior (for example `completed_run_grace_period`, `check_owner`, `store_deadline`, and `disable_storing_incomplete_runs`) without manually editing the `tekton-results-watcher` Deployment.

@adchauha
feat(results): expose Tekton Results Watcher config via TektonConfig
9ef60bb 
Add spec.result.watcher so watcher flags can be configured through
TektonConfig without manually patching tekton-results-watcher Deployment args.
@tekton-robot tekton-robot added the release-note Denotes a PR that will be considered when it comes time to generate release notes. label Jul 13, 2026
@linux-foundation-easycla

linux-foundation-easycla Bot commented Jul 13, 2026 •
edited
Loading

Copy link
Copy Markdown

CLA Signed
The committers listed above are authorized under a signed CLA.

  • ✅ login: adchauha / name: adchauha (9ef60bb)

@tekton-robot

tekton-robot commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
To complete the pull request process, please assign enarha after the PR has been reviewed.
You can assign the PR to them by writing /assign @enarha in a comment when ready.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@tekton-robot tekton-robot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Jul 13, 2026
@jkhelil

jkhelil commented Jul 13, 2026

Copy link
Copy Markdown
Member

/kind feature

@tekton-robot tekton-robot added the kind/feature Categorizes issue or PR as related to a new feature. label Jul 13, 2026
@codecov-commenter

codecov-commenter commented Jul 14, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 25.47%. Comparing base (34adad4) to head (9ef60bb).
⚠️ Report is 12 commits behind head on main.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #3709      +/-   ##
==========================================
- Coverage   25.51%   25.47%   -0.05%     
==========================================
  Files         448      449       +1     
  Lines       23309    23372      +63     
==========================================
+ Hits         5948     5954       +6     
- Misses      16675    16726      +51     
- Partials      686      692       +6
Flag Coverage Δ
unit-tests 25.47% <ø> (-0.05%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

enarha
enarha reviewed Jul 16, 2026
View reviewed changes

@enarha enarha left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The idea of storing the watcher configuration under .result.watcher is good.

The watcher has 21 configuration options (https://github.com/tektoncd/results/blob/main/cmd/watcher/main.go#L64-L85) , but here I see exposed only 15. Was that done on purpose and what's the criteria to decide which flags are exposed through the TektonConfig and which not?

I believe that as implemented, the new flags will be set on new deployment, but if user changes the TektonConfig later, the configuration on the cluster won't be updated. Can you please test that scenario and ensure it works? Updating the TektonConfig should always trigger reconciliation and new configuration should take an effect immediately.

enarha
enarha reviewed Jul 16, 2026
View reviewed changes
Comment thread pkg/reconciler/common/transformers.go Outdated
Comment thread pkg/reconciler/kubernetes/tektonresult/transform_test.go
Comment thread pkg/reconciler/common/transformers.go Outdated
Comment thread pkg/apis/operator/v1alpha1/tektonresult_validation.go
@jkhelil

jkhelil commented Jul 17, 2026

Copy link
Copy Markdown
Member

@adchauha Can you please adress @enarha comments. Thank you

@adchauha
fix(results): address watcher TektonConfig review
e5eb20a 
Sync Watcher on TektonConfig updates, hash pod labels for
free-text values, safely replace two-form container args,
validate watcher on TektonConfig, and expand unit tests.
Signed-off-by: adchauha <adchauha@redhat.com>
@adchauha

adchauha commented Jul 21, 2026

Copy link
Copy Markdown
Author

The idea of storing the watcher configuration under .result.watcher is good.

The watcher has 21 configuration options (https://github.com/tektoncd/results/blob/main/cmd/watcher/main.go#L64-L85) , but here I see exposed only 15. Was that done on purpose and what's the criteria to decide which flags are exposed through the TektonConfig and which not?

I believe that as implemented, the new flags will be set on new deployment, but if user changes the TektonConfig later, the configuration on the cluster won't be updated. Can you please test that scenario and ensure it works? Updating the TektonConfig should always trigger reconciliation and new configuration should take an effect immediately.

It is intentional. Under spec.result.watcher we expose user-facing behavior flags (grace period, store deadline, selectors, timeouts, etc.). Performance knobs (threadiness/qps/burst/disable-ha) already live under spec.result.performance. Connection/install wiring (api_addr, auth_mode) comes from the Results manifest. Token must not live in the CR, use Secrets. Documented in docs/TektonConfig.md / ResultsWatcherProperties.

UpdateResult synced Performance/Options but not Watcher, so later TektonConfig edits could be ignored. Fixed by syncing Spec.Watcher the same way, plus TestUpdateResult_WatcherPropagation. Updating TektonConfig watcher fields now reconciles through to the live Deployment.

khrm
khrm reviewed Jul 23, 2026
View reviewed changes
Comment thread pkg/apis/operator/v1alpha1/watcher_types.go Outdated
khrm
khrm reviewed Jul 23, 2026
View reviewed changes
Comment thread pkg/apis/operator/v1alpha1/watcher_types.go Outdated
khrm
khrm reviewed Jul 23, 2026
View reviewed changes
Comment thread pkg/reconciler/common/transformers.go Outdated
khrm
khrm reviewed Jul 23, 2026
View reviewed changes
Comment thread pkg/apis/operator/v1alpha1/watcher_types.go Outdated
khrm
khrm reviewed Jul 23, 2026
View reviewed changes
Comment thread pkg/reconciler/common/transformers.go
khrm
khrm reviewed Jul 23, 2026
View reviewed changes
Comment thread pkg/reconciler/common/transformers.go
@adchauha
fix(results): use *string and fix negative duration args
ccb9721 
Allow clearing watcher text defaults via empty string pointers,
and correctly replace two-element args whose value starts with '-'
(e.g. -completed_run_grace_period -1h).
Signed-off-by: adchauha <adchauha@redhat.com>
@tekton-robot tekton-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Jul 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@enarha enarha enarha left review comments
@pratap0007 pratap0007 Awaiting requested review from pratap0007
+1 more reviewer
@khrm khrm khrm left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

kind/feature Categorizes issue or PR as related to a new feature. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@adchauha @tekton-robot @jkhelil @codecov-commenter @khrm @enarha